[WIP] CAS draft (adopting to CI/CD, not for review / merge)#2073
Draft
filimonov wants to merge 2523 commits into
Draft
[WIP] CAS draft (adopting to CI/CD, not for review / merge)#2073filimonov wants to merge 2523 commits into
filimonov wants to merge 2523 commits into
Conversation
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
…ent_addressed stateless lanes) Config Workflow check failed with 'Workflows are outdated' for master.yml, pull_request.yml, pull_request_community.yml, release_builds.yml. Regenerated via 'python3 -m praktika yaml'. The regeneration adds the two CAS stateless jobs to the generated workflows: 'Stateless tests (arm_binary, content_addressed storage, parallel)' and 'Stateless tests (arm_binary, content_addressed s3 storage, parallel)' (the rustfs-backed lane). CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=927ea142c9cb14759623861eb004261d0b4b1c8f&name_0=PR&name_1=Config+Workflow PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
…teless lane The lane's start_rustfs expected a pre-extracted binary at ci/tmp/rustfs and failed on CI runners where nothing provisions it (the workflow wipes ci/tmp on every run). Download the static musl build for the runner architecture from the RustFS GitHub release (1.0.0-beta.9) when the binary is absent, mirroring how setup_minio.sh downloads minio/mc. Validated locally: the beta.9 binary passes the conditional-operation semantics the CA pool requires (second 'If-None-Match: *' PUT -> 412, wrong-etag conditional DELETE -> 412, right-etag DELETE succeeds), and download_rustfs provisions an executable binary end-to-end. PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
Fast test fails at cmake generation: 'Target "dbms" links to ch_contrib::crc32c but the target was not found' — the fast-test job initializes a limited submodule list that does not include contrib/crc32c, so the unconditional add_contrib is skipped while the dbms link line still references the target. The dependency is dead: it was wired in for per-block CRC32C in the early CAS run-file format (5f1272c), which was later replaced by the text record-stream codecs; no source file includes the library today. Restore the pre-CAS state: crc32c is built only for google-cloud-cpp, and dbms does not link it. CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=835251f81cb5af73ad9eaa3a835f50f0c8b678db&name_0=PR&name_1=Fast+test PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 17, 2026
Fast test builds without SSL and failed on the unconditional 'openssl/evp.h' include in CasBlobHashingWriteBuffer.cpp. Wrap the OpenSSL-backed Sha256 hashing write buffer and the one-shot digest in '#if USE_SSL'; on non-SSL builds selecting blob_hash = 'sha256' now fails closed with SUPPORT_IS_DISABLED. CityHash128 and XXH3-128 blob hashes are unaffected. PR: #2073 Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 18, 2026
… test regression) CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Fast+test PR: #2073 A CAS parser commit grouped `RELOAD_DICTIONARY`/`RELOAD_MODEL`/ `RELOAD_FUNCTION` with `CONTENT_ADDRESSED_GARBAGE_COLLECTION` into a format case that prints only the optional disk, dropping the reload targets: `SYSTEM RELOAD MODEL my_model` formatted as `SYSTEM RELOAD MODEL` (failed 04117_parser_system_query_variants and 04124_parser_system_query_extra in Fast test). Fold all four types back into the generic target-printing case (table / target_model / target_function / disk else-if chain) — for the CA GC command the disk branch produces the identical output. Both stateless tests verified locally via clickhouse-local against their references. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 18, 2026
…als (arm_tidy, T13 batch 1) CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Build+(arm_tidy) PR: #2073 Removes default arguments from all virtual/override methods flagged by `google-default-arguments` (147 sites: `CasBackend.h` interface, `IObjectStorage.h`/`S3ObjectStorage.h`, all backend implementers, test helpers/fixtures) and adds non-virtual convenience overloads on the base classes that forward the previous default values. Derived classes gain `using` declarations to unhide the base overloads. Qualified parent-implementation calls in test fault backends switched to the explicit 3-arg form — the 2-arg form would now route through the base forwarder and re-enter the derived override virtually (double fault injection; caught by the battery). Bulk edits produced by codex (gpt-5.6-luna) per the T13 brief; overload visibility and qualified-call fixes plus verification by Claude. Battery 919/919 green. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
filimonov
added a commit
that referenced
this pull request
Jul 18, 2026
CI report: https://altinity-build-artifacts.s3.amazonaws.com/json.html?PR=2073&sha=aeb13b24394023fa8cd9d310d4cbcbc308380af1&name_0=PR&name_1=Build+(arm_tidy) PR: #2073 Semantics-preserving conformance for the remaining flagged classes: readability-container-contains, readability-isolate-declaration, google-runtime-int (AWS SDK retry-API overrides keep `long` with targeted NOLINT — the override contract owns the type), readability-duplicate-include, cppcoreguidelines-init-variables, cert-msc, modernize-raw-string-literal, modernize-use-starts-ends-with, bugprone-empty-catch (comments only — no new behavior), googletest naming, bugprone-argument-comment, bugprone-optional-value-conversion, bugprone-misplaced-widening-cast (CasTypes.h site audited: not a real precision bug — the value is range-validated to 0-5; cast made explicit without value change). CasRefCowMap's own `contains` keeps its `find` with NOLINT (self-recursion). Bulk edits by codex (gpt-5.6-luna) per the T13 brief (.superpowers/sdd/task-13-batch2-report.md); one over-removed include (PartFolderAccess.h) restored and verification by Claude. Battery 919/919. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Each test deliberately drives production code into a genuine invariant violation and expects a LOGICAL_ERROR (source-edge ID zero reserved, out-of-order source-edge append, zero-component RefTxnId render, seal sealed_from exceeding snapshot_id, abandon-then-stage lifecycle violation). Correct production behavior in all five; under debug/ sanitizer builds LOGICAL_ERROR aborts before EXPECT_THROW/EXPECT_ANY_THROW can observe it. Converted to EXPECT_DEATH with DB::abort_on_logical_error forced true inside each child (same technique as the part_write.cpp batch) for cross-build-type determinism. Positive controls and the out-of-scope Class-D test (EncodeAllowsExactlyMaxRemovalBytes, same file as RenderRejectsZeroComponent) left untouched. Implemented by codex (gpt-5.6-luna); build/battery independently re-verified by Claude (924/924, 0 failures) after the codex sandbox could not commit (read-only .git). Per docs/superpowers/reports/2026-07-18-logical-error-asan-abort-triage.md. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…s for sanitizer safety Seven tests reach a genuine, correct production LOGICAL_ERROR guard but in a way that plain EXPECT_DEATH wrapping couldn't safely cover: - Three (CasGcRound.OrphanManifestCursorSweepDeletesAndPersistsCursor, CasMountStartup.StaleSelfMountReclaimedAfterWait, CasPoolRemount.ForeignOwnerIsNeverTakenOver) abort INCIDENTALLY during implicit object teardown (MountLeaseKeeper::terminate correctly refusing to release against a foreign incarnation), not as the test's own stated assertion. Restructured so each test's primary behavior keeps asserting normally in the parent process, while the invalid object's entire construction-to-destruction lifecycle is isolated inside its own EXPECT_DEATH -- the parent process never holds or destructs the invalid state. - CasHeartbeat.BackgroundLoopFencesImmediatelyOnConfirmedMismatch throws on a background renewal thread. Default fork-style EXPECT_DEATH forking a multi-threaded process is unsafe and caused a real stall; scoped to gtest's re-exec-based death-test style for just this test (save/restore the flag around it). - Three plain conversions (CasHeartbeat.ForeignTouchMakesRenewThrow, CasMountAudit.KeeperForeignConflictRefusesAndNamesHolder, CasMountLease.KeeperStartAdoptsOurOwnClaimNotDoubleStart) using the same abort_on_logical_error-forcing technique as the earlier batches. Implemented by codex (gpt-5.6-sol, judgment-heavy fixture work); build/ battery independently re-verified by Claude (924/924, 0 failures) after the codex sandbox could not commit (read-only .git). Per docs/superpowers/reports/2026-07-18-logical-error-asan-abort-triage.md. This completes the Class-B remediation (19/19 tests); only Class-D (CasRefCodec.EncodeAllowsExactlyMaxRemovalBytes) remains, pending a live isolated run to determine its cause. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…ismatch (triage #7) The receiver advertised its CAS pool UUID to the sender at request time, but never re-verified it once the disk reservation actually completed. A reservation landing on a different disk (multi-disk policies, tiering, concurrent reservation changes) could be backed by no CAS pool at all, or a DIFFERENT pool than what was advertised -- and the code would still attempt to consume the relink payload for that reservation, risking a relink that points at content in the wrong pool. Capture the advertised pool UUID into a local at both advertise sites, and re-check it against the actually-reserved disk's CAS pool immediately before consuming the relink payload: any mismatch (no CAS exchange at all, or a different pool UUID) now falls back to a plain byte fetch, with a LOG_INFO explaining why. This also replaces the previous unconditional for a non-content-addressed target disk with the same fallback -- per the fix contract, neither failure mode should abort the fetch, both should degrade to bytes. No new unit test: this guard needs a live multi-disk fetch to exercise meaningfully; runtime coverage is deferred to the R5 scenario campaign (S38-style integration scenario). Validated by full build + the CAS gtest battery staying green. Implemented by codex (gpt-5.6-sol); build/battery independently re-verified by Claude (full clickhouse binary + 924/924 unit battery) after the codex sandbox could not commit (read-only .git). Per docs/superpowers/reports/2026-07-17-codex-review-triage.md §3.7. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…ured (triage #16) tryBuildReaderExecutor's fallback condition checked distributed_cache, memory_cache, filesystem_caches, decryption_stages, and async_prefetch, but not file_view -- a configured byte-window stage would be silently dropped rather than triggering a fallback to the legacy read path, since the executor doesn't implement it. Added `file_view` to the condition and to the fallback log message. Full Cas* battery + IO/ReadPipeline/ReaderExecutor/FileView unit tests: 924/924 + 70/70, zero failures. Per docs/superpowers/reports/2026-07-17-codex-review-triage.md §2.16. Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
#1) Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…reation, event sink installed before threads (triage #8, #10) runGarbageCollectionRoundNow/runOneGcRoundForTest now hold gc_scheduler_mutex for the whole round (mirroring gcHealth), instead of snapshotting a raw CasGcScheduler* and releasing the lock before calling it -- the previous shape let shutdown()'s gc_scheduler.reset() race the escaped pointer. A new shutdown_called flag (TSA_GUARDED_BY the same mutex) stops the lazy-creation path from resurrecting a scheduler after shutdown has begun. Scheduler construction now reads the already-guarded cas_store member directly instead of recursively calling store() (which also takes gc_scheduler_mutex), so holding the mutex across construction can't self-deadlock. store() now returns a Cas::PoolPtr by value (a shared-ownership snapshot taken under the same mutex) rather than `const PoolPtr&` into the live member; partAccess()'s null-check-and-return is now guarded by the same mutex too. Both cas_store and part_access were previously read/reset with no synchronization at all, racing shutdown()'s unlocked resets. Separately (triage #10): CasEventSink is now threaded into PoolConfig and installed on the Pool before Pool::open/openForDecommission ever call mountWritable, which spawns the mount's renewal thread. Previously the sink was installed via setEventSink() after open() returned, so an event fired during the open call itself (or by a very fast renewal thread) was silently dropped by the still-null sink, and the assignment raced the thread's reads of event_sink_. CasPool::setEventSink() remains for tests/pre-open wiring only, documented as such. Verified: full clean rebuild of clickhouse + unit_tests_dbms, and the CAS gtest battery (Cas*:CA*:ContentAddressedLog*:CountingBackendShape*:RefSnapshotCodec*:RefTableCacheEviction*:RefWriter*) at 924/924 passing, independently of the implementer's own report. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 3.8, 3.10 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
-narrow) If a transaction stages new content for a part (create) and then removes that same part's directory before commit, removeDirectory only cleared content_removed marks -- the staged manifest entries and any in-flight PartWriteTxn build were left behind. The entries would never be published (the ref-drop wins), but the build's precommit/blob edges had nothing left to ever publish or abandon them. removeDirectory now also clears st->entries and, when a build is present, calls abandon() and resets it, so a same-transaction create-then-remove publishes nothing and leaves no live precommit. Verified: full clean rebuild of unit_tests_dbms and the CAS gtest battery at 924/924 passing, independently of the implementer's own report. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.12 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…f deleting it (T5, triage #9) Prior hardening rounds fenced the decommission tail's mount-lease and epoch-counter deletes to the exact tokens/values observed under the decommission claim, closing the successor race for both. The owner anchor (OwnerObject) resisted the same approach: it holds only server_uuid, which is byte-identical whether written by the decommissioned run or by a legitimate successor reclaiming the same srid, so no amount of delete-side token fencing can tell them apart. Decision (user-confirmed): stop deleting the owner object. Decommission's last step becomes a conditional rewrite in place (retired_at_ms tombstone) using the same exact-token conditional-write machinery already used elsewhere in this file. A racing successor's own write simply fails the conditional overwrite -- no bespoke protocol, ordinary CAS semantics settle it. claimOwnerOrThrow refuses to silently resume a tombstoned identity. This intentionally does not chase the last microsecond of the decommission-vs-concurrent-recreate race; it removes the destructive half (a live successor anchor being deleted out from under it). Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 3.9 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…#25) If commit() threw (some parts already published, compensating rollback ran best-effort), nothing prevented a caller from calling commit()/tryCommit() again on the same transaction object -- a second attempt would re-iterate parts and call publishStaging again in an inconsistent, never-tested state. Added a failed flag, set in the catch block before the rollback loop runs (so it's set even if the rollback loop itself throws); commit() now refuses outright with LOGICAL_ERROR if a prior attempt already failed. tryCommit delegates to commit(), so the one guard covers both entry points. Verified: rebuild of unit_tests_dbms and the CAS gtest battery at 924/924 passing, independently of the implementer's own report. The existing CaWiringWrite.PartialCommitRollsBackPublishedParts test (the closest existing coverage for the rollback path) also passes unchanged. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.25 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
truncateFile silently pretended to succeed without ever touching the file -- content-addressed blobs are immutable, so a caller relying on a real truncate got silent data corruption (the file kept its old content/size). Replaced the no-op with a NOT_IMPLEMENTED throw explaining why (blobs are immutable; whole-file rewrites replace the staged entry instead). Verified: rebuild of unit_tests_dbms and the CAS gtest battery at 924/924 passing, independently of the implementer's own report. Added CaTransactionOps.TruncateFileIsNotSupported asserting the thrown error code. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.23 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…(triage #9) Prior hardening rounds fenced the decommission tail's mount-lease and epoch-counter deletes to the exact tokens/values observed under the decommission claim, closing the successor race for both. The owner anchor (OwnerObject) resisted the same approach: it holds only server_uuid, which is byte-identical whether written by the decommissioned run or a legitimate successor reclaiming the same srid, so no amount of delete-side token fencing can tell them apart. OwnerObject gains an optional retired_at_ms tombstone (omitted from the encoded bytes when unset, so a never-retired owner's on-disk format is byte-identical to before). Decommission's final step is now a conditional rewrite (putOverwrite against the exact token it just read) that sets the tombstone instead of deleting the object; a racing successor's own write simply fails the conditional overwrite and the tail aborts closed with a "successor reclaimed" warning, exactly like the existing mount/epoch fencing. claimOwnerOrThrow refuses to silently resume a tombstoned identity even when the uuid matches, with a message distinct from the existing foreign-owner case and manual-recovery guidance (clear the tombstone and restart) mirroring the anchor-lost case already documented there. report.slot_removed keeps its name and truthiness semantics (mount+epoch deleted, owner tombstoned) since ~14 existing assertions depend on it; only its doc comment changed. This deliberately does not chase the last microsecond of the decommission-vs-concurrent-recreate race -- per-user direction, that scenario is not the priority. It removes the destructive half: a live successor anchor being deleted out from under it. Design note: docs/superpowers/specs/2026-07-18-t5-owner-tombstone-design.md (804cbbf). Verified: full clean rebuild of clickhouse + unit_tests_dbms (after two rebuild attempts were externally interrupted mid-build with no relation to this change; the third attempt completed cleanly) and the CAS gtest battery at 928/928 passing, independently of the implementer's own report. Reviewed the format, decommission, and claim-side diffs personally, including the dedicated SuccessorOwnerRewriteBeforeTombstoneBackend fixture that exercises the exact race deterministically. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 3.9 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…tity field (triage #20a)
decodeMountLease and decodeGcHeartbeat had no check that their identity
fields ("su"/"we" and "by"/"seq" respectively) were actually present in the
decoded body -- an absent field silently left the corresponding struct
member at its default-constructed value instead of failing on
corrupted/truncated persisted data. Mirrors the saw_* + throw shape
decodeOwner already used in the same file family.
Verified: rebuild of unit_tests_dbms and the CAS gtest battery at 930/930
passing, independently of the implementer's own report.
Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 3.20
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…an requested (triage #20c) decodeFoldSeal never cross-checked the decoded body's own generation field against the (generation, attempt) pair encoded in the object's key -- a corrupted or mis-keyed store object would silently be accepted as if it were the requested generation. Added an optional expected_generation parameter (default nullopt, so the existing debug-tool and test call sites need no change) that, when passed, throws CORRUPTED_DATA on a mismatch. The three production call sites that already know the requested generation (Gc::readFoldSeal, CasOrphanManifestSweep::sealedRefCursor, CasFsck's invariant loop) now pass it; CasInspect.cpp's arbitrary-key debug render is intentionally left unchanged. Verified: rebuild of unit_tests_dbms and the CAS gtest battery at 931/931 passing, independently of the implementer's own report. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 3.20 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…d of narrowing (triage #21) The three header "v" fields were read as a u64 and narrowed with a plain static_cast<uint32_t>, so a persisted value like 4294967299 (2^32 + 3) silently became 3 and passed as if it were a normal, low format version. Added JsonObjectReader::readU32Number, which reads through readU64Number and throws CORRUPTED_DATA if the value exceeds uint32 range. Replaced the three header-version narrowing sites (CasTextFormat.cpp, the cas_run header in CasRecordStreamFormat.cpp, the blob envelope header in CasBlobEnvelopeFormat.cpp) with calls to it. Left every other readU64Number call in these files unchanged -- they're genuinely u64 fields (sizes, counts, timestamps, the blob provenance ch_version), not narrowed header versions. Verified: rebuild of unit_tests_dbms and the CAS gtest battery at 931/931 passing, independently of the implementer's own report. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.21 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…ish (triage #13) promote()'s BuildPublish and RefRepoint events fire AFTER store->appendRefOps has already durably committed the promotion. A throwing sink (e.g. a system-log queue push failing under OOM) at either point turned an already-durably-succeeded promote into a caller-visible failure, even though the promotion itself is fine and idempotent re-drive already handles retries correctly. Both post-durable emissions are now wrapped in try/catch(...) that logs and swallows the failure via tryLogCurrentException. The BlobReuseAdopt emission inside the appendRefOps closure is unchanged -- it fires while the durable append is still being computed, before it commits, so a throw there correctly aborts the operation. Verified: rebuild of clickhouse + unit_tests_dbms and the CAS gtest battery at 933/933 passing, independently of the implementer's own report. Includes a fault-injection regression test (a sink that throws only for BuildPublish) confirming promote() no longer propagates the sink's exception and the ref still resolves to the promoted manifest. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.13 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…epting a nonexistent path (triage #24) Both if_exists and should_remove_objects were discarded parameters -- if_exists was never checked, so unlinking a path that doesn't exist anywhere (not staged, not committed) silently succeeded regardless of what the caller asked for. All three unlinkFile branches now check existence before mutating state: the part-file branch checks the committed manifest via the same getView idiom publishStaging already uses (view->hasFile); the verbatim table-file branch checks getNamespaceFile; the loose mountpoint branch checks getMountpointObject. Absence with if_exists=true is a no-op; absence with if_exists=false throws FILE_DOESNT_EXIST. should_remove_objects stays unused -- this fix doesn't need it. Verified: rebuild of clickhouse + unit_tests_dbms and the CAS gtest battery at 933/933 passing, independently of the implementer's own report. Includes a regression test covering all three if_exists/exists combinations for the part-file branch. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.24 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…triage #28) If PartWriteTxn's constructor threw, the build-seq allocateBuildSeq() had just minted was never retired -- retireBuildSeq is only called on publish/abandon/dtor of a successfully constructed and registered build. A construction failure permanently pinned the GC watermark floor at that seq forever. Added a SCOPE_EXIT that retires the seq unless a registered flag was set (only set after registerInflightBuild succeeds), mirroring the same dismissed-flag SCOPE_EXIT idiom already used elsewhere in this codebase (ContentAddressedTransaction.cpp's staged-temp-file cleanup). Verified: rebuild of clickhouse + unit_tests_dbms and the CAS gtest battery at 933/933 passing, independently of the implementer's own report. Includes a fault-injection regression test (a sink that throws on BuildStart, forcing construction to fail) confirming the failed seq no longer pins minActive and the next build gets the following monotone sequence. Triage doc: docs/superpowers/reports/2026-07-17-codex-review-triage.md sec 2.28 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
Blob body uploads are fault-tolerant: they run through the pool's shared CasRequestController, which resolves an ambiguous S3 response (SlowDown, 429, 5xx) by re-reading the key and reissuing within a budget. The blob freshness-meta sidecar (putMetaIfAbsent/casMeta in CasBlobMeta.cpp) was not on that path -- it called backend.casPut directly, with a single SDK attempt. Under S22's fault injection (fault_rate=0.2), a SlowDown on a .meta PUT had a flat ~20% chance of escaping to the INSERT/merge client as HTTP 500 (S3_ERROR). Added two new controller primitives (CasRequestControl.h/.cpp): - putOverwriteControlled: a controlled If-Match overwrite for a mutable marker, resolving an ambiguous attempt with one GET -- unchanged token means the attempt never applied (retry); matching bytes means an earlier attempt already landed (Committed); anything else is a genuine competing write (Conflict, returned as a value, never thrown). - putIfAbsentControlledMutable: the create-side sibling. The existing putIfAbsentControlled deliberately was not reused here -- its resolve (resolveByExactGet) throws CORRUPTED_DATA on any different bytes at the key, which is correct for the ref-log lane's immutable content-addressed keys but wrong for a mutable state marker, where a pre-existing different value (e.g. a stale Condemned marker still present when a vanished blob body is freshly re-uploaded) is a normal, expected outcome, not corruption. This gap surfaced as a real battery failure (CasPartWriteTxn.PutBlobResurrectVanishedReUploadsHeldBody) during implementation and was fixed by adding the dedicated primitive rather than weakening the ref-log lane's contract. Both are exposed through CasRefLedger/Pool wrappers (stagingConditionalOverwrite, stagingPutIfAbsentMutable) mirroring the existing stagingPutIfAbsent/ stagingConditionalCreate pattern. CasBlobMeta.cpp's putMetaIfAbsent/casMeta now take a Pool& and route through these; every production call site (CasPartWriteTxn.cpp's fresh/resurrect/adopt-backfill writers, CasGc.cpp's condemn-marker writer) was updated. loadMeta stays a plain GET; deleteMetaExact is deliberately left unchanged (its sole caller is a GC condemn-cleanup path, not the S22 hot path, and GC's own outer-round retry/idempotence model already tolerates a failed cleanup delete as a silent no-op). Verified: full clean rebuild of clickhouse + unit_tests_dbms and the CAS gtest battery at 938/938 passing, independently confirmed (not just the implementer's report) after finding and fixing the putIfAbsentControlled reuse gap myself. RCA: docs/superpowers/reports/2026-07-18-s22-throttle-retry-rca.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…er blocks behind a GC round Final whole-branch review of the fix wave (docs/superpowers/reports for 2026-07-17/2026-07-18) found two real gaps in T9's lifecycle-locking fix: 1. Critical: partAccess() still returned `CachedPartFolderAccess&` obtained from a raw part_access.get() taken under gc_scheduler_mutex, then released before the caller's first dereference. shutdown() could acquire the mutex and destroy the object in that window -- the exact UAF class T9 was meant to close, just moved from store() (already fixed as a by-value shared_ptr snapshot) to partAccess() (missed). Fixed identically: part_access is now a shared_ptr, and partAccess() returns a by-value snapshot taken under a lock. Every call site (36 across ContentAddressedMetadataStorage.cpp and ContentAddressedTransaction.cpp) already used the temporary in one full expression (`partAccess().method(...)`), so this is a mechanical `.` -> `->` rewrite with no caller logic changes. 2. Important: gcHealth() -- an unprivileged SELECT on system.content_addressed_mounts -- held gc_scheduler_mutex for its whole call, which a manual GC round (runGarbageCollectionRoundNow/ runOneGcRoundForTest) ALSO holds for its entire, unbounded-duration scan-and-delete workload. A health query could therefore stall behind an in-progress round. The scheduler's own gcHealth() is lock-free (atomic reads plus wedgedRefLaneCount), so serializing it behind the round bought no safety. Split the single lifecycle mutex into two: gc_scheduler_mutex now serializes ONLY one round at a time and makes shutdown wait for an in-flight round to finish (unchanged priority: clean completion over fast shutdown); a new pointer_mutex guards brief reads/writes of cas_store/part_access/gc_scheduler themselves (snapshot, create-if-absent, reset) and is never held across a round or a caller's use of a returned snapshot. gc_scheduler is now a shared_ptr for the same reason as part_access: a round takes its snapshot under pointer_mutex, releases it, and runs via the snapshot -- safe even if shutdown concurrently resets the member, since the snapshot's own refcount keeps the object alive. startup() is marked TSA_NO_THREAD_SAFETY_ANALYSIS: it runs exactly once, single-threaded, strictly before this object is exposed to any other thread, so the pointer_mutex/gc_scheduler_mutex discipline (which exists for concurrent access after startup) does not apply and would only add meaningless lock/unlock pairs. Verified: full clean rebuild of clickhouse + unit_tests_dbms and the CAS gtest battery at 938/938 passing. Final review: .superpowers/sdd/final-whole-branch-review.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…tion instead of discarding it
Final whole-branch review of the fix wave found that T12's S22 controller
plumbing (putMetaIfAbsent/casMeta now return Committed/Conflict/Unresolved)
was wired to the controller correctly, but its two real callers in
uploadFromSource discarded the contract:
- writeFreshMetaClean ignored putMetaIfAbsent's result entirely. Its
comment claimed a Conflict there only ever means "a racing writer
already created the same Clean state" -- but a stale Condemned marker
can genuinely be present at this exact call site (a body that vanished
and was freshly re-uploaded), proven reachable by
CasPartWriteTxn.PutBlobResurrectVanishedReUploadsHeldBody. Left
unhandled, that marker stayed Condemned forever, misleading every
future point-reader.
- writeResurrectMetaClean's 8-attempt outer loop already reloaded and
retried on any non-Committed result, but silently gave up after
exhausting all 8 attempts ("best-effort... leave the meta as-is") --
contradicting the RCA's explicit requirement that budget exhaustion
reach the caller as a controlled retry signal, not a silent skip.
writeFreshMetaClean now delegates to writeResurrectMetaClean with no
prior point-read, so a fresh upload gets the SAME reload-and-reconcile
handling a resurrect already had. writeResurrectMetaClean's loop now
throws throwCasWriteRetryLater (NETWORK_ERROR, the same signal used
elsewhere on this write path) after exhausting all attempts instead of
silently succeeding with a stale marker.
Verified: full clean rebuild of clickhouse + unit_tests_dbms and the CAS
gtest battery at 939/939 passing. Extended
PutBlobResurrectVanishedReUploadsHeldBody to assert the meta ends up
Clean (previously unchecked); added
PutBlobFreshMetaExhaustionThrowsRetryLater covering the exhaustion path
with a persistently-faulting backend, confirming the body PUT is
unaffected and only the meta write's failure surfaces.
Final review: .superpowers/sdd/final-whole-branch-review.md
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF
Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
… instead of failing outright Final whole-branch review found that the owner-tombstone step (T5, triage #9) used a bare pool_backend->putOverwrite(...) wrapped in a plain catch(...): any exception at all -- including one where the write actually landed and only the response was lost -- was reported as a hard failure (report.slot_removed = false), with no way to tell "genuinely didn't apply" from "applied, ack lost". A retry in the latter case has no way to resolve the ambiguity either. The owner-tombstone attempt now goes through a standalone CasRequestController (decommission is an administrative, non-hot-path operation with no mount-lease fence of its own -- the exact-token CAS itself is the safety mechanism, same as the mount/epoch deletes right above it) calling putOverwriteControlled, the same primitive T12 added for the S22 freshness-meta fix. An ambiguous attempt is now resolved with one GET: unchanged token means the write never applied (was never the concern here since decommission doesn't retry within one invocation, but correctly reported as Unresolved rather than a hard failure); matching bytes means this exact tombstone already landed (Committed, not a failure); a genuinely different owner (a real successor reclaim) is Conflict, preserving the existing "successor reclaimed" warning unchanged. This closes the practical resumability gap for the common case (a transient transport blip after the write landed); it does not attempt to solve the deeper "a retry's own claim path meets an already-tombstoned owner mid-decommission" edge case, which would require changes to the claim/open path itself, not just this write -- left as a known, narrow residual risk consistent with the "no big complications" scope already set for T5's related decisions. Verified: full clean rebuild of clickhouse + unit_tests_dbms and the CAS gtest battery at 940/940 passing. Added CasDecommission.OwnerTombstoneAmbiguousSuccessResolvesToCommitted, a backend that throws immediately after the write actually lands, confirming the fix resolves it to Committed instead of the old unconditional-failure behavior. Final review: .superpowers/sdd/final-whole-branch-review.md Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LN7m5KvCqwnfutnPDNUncF Signed-off-by: Mikhail Filimonov <mfilimonov@altinity.com>
…m refutation) CaB140DangleFaithful.tla proved a negative about a mechanism that no longer exists at all: with producers faithful to what the first-phase code actually did (no marker-retaining strip, no field-mixed generation adoption), the first-phase fix mechanism does not exhibit the journal-trim dangle. Both the mechanism it exonerates and the unfaithful reproduction it superseded (CaB140Dangle.tla, removed earlier) are gone; the incident record and the actual fix proof (trim-gate + cursor-in-snap 2x2 matrix) live in CaB140DangleMerge.tla, which stays. A refutation of a dead mechanism gates nothing and cannot regress; full text remains in git history.
…wedge) CaManifestSweepWindow.tla proved that the orphan-manifest sweep must skip a committed body whose removal record is appended to the shard journal but not yet sealed by the GC fold -- deleting it early leaves the removal-fold unable to emit its in-degree decrement, wedging the fold forever. The property involves almost no concurrency: the only ordering that matters (drop ref -> watermark-eligible -> sweep before seal) is constructed directly and deterministically by the unit test CasOrphanManifestSweep.PendingCommittedRemovalBodyIsSkipped (src/Disks/tests/gtest_cas_orphan_manifest_sweep.cpp), and ten sibling sweep tests cover the surrounding behaviors (owned-body skip, budgets, cursor paging, late-log windows). With the interleaving space this small, TLC explores nothing the test does not already pin, so the model added no assurance beyond executable documentation; the prose record stays in the docs. The dedicated runner run_sweepwindow.sh goes with the model. Full text remains in git history.
…r the fix) CaGcResurrectReuploadOrphan.tla reproduced a real leak: a condemned blob replaced by a resurrect re-upload (new token, same content hash) was never re-condemned, because closeBlob keyed the already-retired decision on the hash alone and the touch-gated fold never revisits the blob once its edges are folded. Its job is done: the fix (settle the stale entry AND re-condemn the current token, matching the canonical model's GRetire) landed in CasBlobInDegree.cpp, and the deterministic unit tests CasGcLeak.ResurrectReplacedIncarnationReclaimed / ResurrectReplacedReclaimIsIdempotent / ResurrectReplacedTokenIsCondemnedInMeta (src/Disks/tests/gtest_cas_gc_leak.cpp) pin the scenario and its edge cases directly. At 194 distinct states TLC explored essentially that one scenario, so as a regression gate the model added nothing beyond the tests; the lasting lesson -- the canonical model idealizes the fold as un-touch-gated, which is why it missed this class -- is recorded as prose in the docs. Full model text remains in git history.
…mmit + per-blob upload) Targets the measured ~7.6x CAS-on-S3 insert slowdown (BACKLOG root causes #1/#2): two-level parallelism over the shared IO pool engages ref-ledger batching and overlaps blob uploads, byte-identical + invariant-preserving. TSan gate + the CasRefBatchFlushes-vs-Mutations success metric. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…closed in every consumer One TU-local classifier (4 legal shapes, else CORRUPTED_DATA) now feeds both the state machine and manifestEdgesOfTxn, so an op shape the writer cannot produce can no longer acquire accidental GC edge semantics. The GC fold treats an unrecognized shape exactly like an undecodable body: ref folding aborted for the round, cursor pinned, no destructive work. H2 rewritten as the representable duplicate-removal hazard; new GC integration test pins the abort path. Consult-recommended design (tmp/consult3-answer.md). Gate 1107/1107; writer-path benches within ±5% (absolute medians). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ible through StorageProxy
IStorage's default throws NOT_IMPLEMENTED ("Table engine ... doesn't
support mutations"), so a lazy StorageTableProxy rejected every mutation
while forwarding mutate itself. Found by a ca-soak run reaching the
mutations stage; consulted (see
docs/superpowers/reports/2026-07-21-storageproxy-forwarding-audit.md):
checkMutationIsPossible belongs on generic StorageProxy;
checkTableCanBeRenamed added on StorageTableProxy only (a table-function
proxy must not materialize on metadata-only rename); checkTableCanBeDetached
deliberately NOT added (dictionaries-only hook, lazy excludes them).
Stateless test 05021 pins the UPDATE path; MATERIALIZE TTL via a lazy proxy
remains broken differently (proxy cached metadata lacks TTL) — tracked in
the CAS backlog under the lazy_load_tables decision item.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s under quarantine decision Three product bugs of the unforwarded-virtual class blocked or would block soak stages; the feature is orthogonal to the ref-ledger criterion this round's soak measures. Documented cost: outage-at-load scenarios lose the per-query-retry property until the feature decision lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…alache support
CaIncarnationProofCore.tla proved an Apalache inductive invariant
(IndInv, 19 conjuncts) for the single-leader, token-only fragment of
the GC core. Two independent reasons to drop it, surfaced by the model
audit:
1. It does not model the current design: it predates the namespace-
registry and evidence-staleness amendments, which are covered by
the canonical CaIncarnationCore.tla (bounded TLC, current).
2. It cannot be verified -- or re-derived -- in this environment: no
Apalache binary is installed (tmp/apalache/bin/apalache-mc is
absent), so 'bring the model in line with the code and re-check' is
impossible here.
A stale inductive proof of a superseded fragment that cannot be
re-checked provides false comfort -- the same failure mode that made
CaMetaAbsenceClean worth removing. Apalache.tla (the standard support
module, extended only by the proof core) and run_apalache.sh go with it.
To revive an inductive proof, install Apalache and re-derive IndInv
against the current CaIncarnationCore.tla. Full text remains in git
history; the prose record stays in docs/superpowers/cas/06-tla-models.md
Area 1.
…in-degree design) Model audit (2026-07-22) vs the shipped cas-gc-rebuild code: CaGcIndegRefoldCore.tla proved that the completion-seal cursor must be persisted past what recheck already folded, so that a NON-IDEMPOTENT integer in-degree delta stream (prior count + sum of +/-1 journal deltas) does not re-absorb a removal and drive the counter negative (merged < 0 -> CORRUPTED_DATA). The shipped fold no longer works that way: Gc in-degree is computed by an IDEMPOTENT two-cursor presence-set merge keyed by (blob_hash, source_id) -- 'prior present + activate => present; any remove => absent' (CasBlobInDegree.cpp:380-389) -- and the surviving-edge count is a uint64_t that structurally cannot underflow; there is no merged<0 guard. The model existed precisely to catch the integer-underflow that an idempotent edge-set recompute cannot express, but the code adopted exactly that idempotent recompute, so the hazard is now structurally impossible and the model (its three code citations all drifted) describes a design the code abandoned -- the same situation as the removed EBR CaGcCore.tla. Not a CODE-RISK: the property it guarded holds by construction. Prose record kept in 06-tla-models.md Area 8; full text in git history.
… shipped advisory meta) Model audit (2026-07-22) vs shipped cas-gc-rebuild code. CaMetaDescriptor proved INV-META-BODY (per-hash meta present => body present) treating the meta as THE lifecycle linearization point with meta-first (top-down) delete. The shipped meta contradicts this: Formats/CasBlobMetaFormat.h states the marker 'is only a point-read hint, not the linearization point for blob lifetime ... reads of the blob never consult the meta' -- the body's in-body incarnation_tag + exact-token delete is the real linearizer. GC deletes the body first (CasGc.cpp:419) and drops the meta only advisorily; absent reads identically to Clean (no tombstone). So a Condemned meta legitimately outlives its deleted body -- a state the model forbids. A model whose headline invariant holds in the model but is false in the code is misleading, the same failure mode that made CaMetaAbsenceClean worth removing. The meta's real (advisory) behavior is already gated by CURRENT models: the writer adopt-gate point-read (Condemned => re-upload) by CaEdgeBeforeObserve (K1), and the GC condemn-marker graduation gate by CaGcCondemnMarkerGate; the freshness argument is discharged against CaIncarnationCore. Not a CODE-RISK -- the meta is advisory and never authority for deletion. The audit also found the model's 8 configs omit CHECK_DEADLOCK FALSE, so a spurious terminal-state deadlock in a shallow BFS branch halted TLC before the counterexample was reached -- five of seven sabotages silently reported a deadlock instead of the intended violation (verified by re-running with deadlock checking off: reduced GREEN, all seven sabotages VIOLATE). Moot with the model removed, but recorded. Prose record kept in 06-tla-models.md Area 13; full text in git history. The runner run_meta.sh goes with the model.
CaB140DangleMerge_{buggy,buggy3,fixed,fixed3}.cfg are leftover configs
from an earlier revision of the module. They set TrimGated / MaxGen and
reference INV_NO_GC_LOSS, but do NOT assign the current constant
CursorInSnap, so TLC fails immediately: 'The constant parameter
CursorInSnap is not assigned a value by the configuration file.' They no
longer run against the current module and gate nothing.
The live, canonical config set for this model is the 2x2 matrix
m_both_buggy / m_cursorskip / m_trimonly / m_merged (each assigns both
TrimGated and CursorInSnap), verified working during the model audit
(e.g. m_both_buggy and m_trimonly reproduce the INV_NO_LOSS
counterexample as documented). These are the configs named in
06-tla-models.md Area 5. Removing the four dead configs; full text in
git history.
…g death-tests, ReadBufferFromMemory UBSan guard) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HKgdqVjZwkpWPxLyHzduPb
…-reclaim mechanism Model audit (2026-07-22): CaCasMountCore.tla is faithful to the shipped code, but the deep-dive doc's Area 4 was materially stale. It listed the deleted config CaCasMountCore_sab_supersededwrites.cfg (retired when the per-write body-read guard was removed) and omitted the entire lease-boundary-exclusivity extension the code actually uses. Rewrite Area 4 to the current mechanism: observation-based reclaim (wait TTL+Drift on the reclaimer's OWN monotonic clock observing a stable holder token, never trusting the foreign body's wall-clock timestamp -- claimMountAwaitingExpiry, CasServerRoot.cpp:395-470), reclaim installs the successor's own body (CasServerRoot.cpp:331-332), epoch is a separate CAS-bumped ServerEpoch that fails closed not resets (allocateWriterEpoch, :151-195), and the per-write path is a pure-local liveness check (CasMountRuntime::mayMutate, CasMountRuntime.cpp:62-66). Update the file/config list, the invariant/sabotage table (add sab_wallclockreclaim as the negative control for trusting the foreign wall clock, and the rev.6 witnesses), and the summary-table row. The model itself is unchanged and CURRENT.
…part commit Rewritten after two independent concurrency reviews (Fable + Codex gpt-5.6-sol) refuted the nested same-pool fan-out (deterministic IO-pool deadlock) and the standard-S3-path investigation showed the proven pattern. Simpler: bounded part-worker concurrency engages ref-ledger batching and gets blob overlap from WriteBufferFromS3 multipart; no nested same-pool waits; commit pool disjoint from getThreadPoolWriter; dropRefIfMatches rollback; semantic-equivalence determinism; pool-size-1 saturation gate. Intra-part blob fan-out deferred. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ter models Model audit (2026-07-22) vs shipped cas-gc-rebuild code. The branch rearchitected GC/writer (ref-log + source-edge runs + round-only pacing + advisory freshness meta), and several models' 'CURRENT' notes over-claimed fidelity. No CODE-RISK anywhere -- in every case the code upholds the proven invariant via an equal-or-stronger mechanism -- but the notes now state precisely what drifted: - CaIncarnationCore: reframed as the architecture-independent safety spine (invariants map to code), with its concrete manifest-embedded journal/fence structure marked superseded by the ref-log/source-edge architecture. - CaBuildRootPrecommit: the 'inline closure at precommit' bullet is factually wrong for shipped code (lazy manifest-body fold at GC time + clamp barrier + GC-sole-deleter); 'presence re-check' is actually owner-liveness fail-close (stronger). Conclusion still holds. - CaEdgeBeforeObserve: no-tokened-reval + edge-order + K1 MATCH; K3Head/K3AdoptCheck model a promote-time tokenless probe the code removed (tokenless leaf now manifest-trusted, in-degree-pinned). - CaGcAckFloorCore: relabelled MIXED -- its graduation gate is the superseded writer-ack floor (code paces on condemn_round < current_round, CasGc.cpp:1867); GRebuild + clamp-suppression still match and are its unique current value. - CaGcRootLocalPartManifestCore: note that two fence-era sabotages (sab_reducerownsfence, sab_crosssharddisplacement) now crash TLC (RuntimeException) rather than cleanly violating -- broken, not merely superseded; part of the deferred fence/recheck excision. Updates the per-model currency prose and the summary-table status column. Model .tla logic unchanged; realignment rewrites are scoped follow-ups (false-green risk, deferred).
Record the model-audit outcome in the directory README: add an Audit note section (headline: no CODE-RISK -- the code violates no proven rule; the cas-gc-rebuild rearchitecture left a cluster of models gating pre-rebuild mechanisms MIXED/drifted while their safety conclusions still hold), a MIXED status-legend entry, and precise summary-table statuses for CaIncarnationCore (safety spine; structure superseded), CaBuildRootPrecommit (mechanisms drifted), CaEdgeBeforeObserve (K3Head/K3AdoptCheck drifted), and CaGcAckFloorCore (graduation gate superseded; GRebuild+clamp still match). The realignment rewrites are called out as deferred (false-green risk on concurrency proof models).
…udit 2026-07-22)
The model sweep found that all three EnableSharding configs crash TLC
with a RuntimeException (CHOOSE m in {} at TheM, .tla:113) reached from
the GReduceShard/scatter path -- a both-empty edge (e.old=e.new={})
reaches the IF e.old#{} THEN TheM(e.old) ELSE TheM(e.new) manifest
extraction. This hits not only the two fence-era sabotages
(sab_reducerownsfence, sab_crosssharddisplacement) but the POSITIVE gate
stage5_sharding, which historically ran 983.9M states and now dies at
158 -- a regression, so the sharded scatter/reduce machinery currently
has no working positive gate in this model. Sharding correctness stays
covered by gtest_cas_gc_shard_plan.cpp. Fixing the model's sharding arm
is proof-model surgery folded into the deferred realignment; the
non-sharding stages/sabotages are unaffected. Documented in Area 7 and
the README audit note.
…place Both COW containers (RefCowMap, RefCowManifestSet) now fold the overlay into their base IN PLACE when the base is uniquely owned (`use_count() == 1`), eliminating the per-flush O(N) base copy the 20-min soak profile flagged as the top CAS-compute stack (`RefCowManifestSet::materialize` unordered_set copy inside `CasRefLedger::flushRefBatch`). A base still shared with a copy keeps the old build-fresh-and-swap path, so a shared holder's view stays byte-unchanged. The member is now `shared_ptr<Base>` (non-const) so the in-place fold needs no const_cast; every external surface keeps const semantics. Safety argument (in `materialize`'s doc): the container is not thread-safe by contract, and a `use_count()==1` seen by the sole owner cannot concurrently rise, so the in-place fold has no other observer. TDD: new tests first (both suites) -- MaterializeReusesBaseWhenUniquelyOwned (fast path reuses the base allocation), MaterializeBuildsFreshBaseWhenBaseIsShared (the load-bearing pin: shared holder's view unchanged), plus empty-overlay no-op. RED build failed exactly the two reuse tests; GREEN gate 1113/1113, 0 failures. Benchmark: added BM_FlushInstallUniqueOwner (models the production flush, which is the uniquely-owned case). Existing BM_FlushInstall/BM_Materialize copy a shared fixture so they stay on the slow path by construction and do NOT change -- reported honestly. Within-run (same machine) cpu, slow vs fast path: N=100 23,623 ns -> 2,518 ns (~9x) N=1,000 239,443 ns -> 4,134 ns (~58x) N=10,000 2,618,976 ns -> 13,371 ns (~196x) N=100,000 46,426,524 ns -> 27,375 ns (~1700x) Slow path is O(N.lgN); the fast path grows ~11x over a 1000x N range (residual is apply's O(lgN) map work + a Pause/Resume floor), i.e. the O(N) copy is gone. BM_ScratchCopy stays O(1) ~60-75 ns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Y1husTcX5kgM8YjpcDxeM
Model audit (2026-07-22) turned up two stale citations in comments (not logic): - CasGc.h (x2): the fold is described as a 'three-cursor merge', but the retired-list-into-the-run refactor made it a two-cursor merge -- the prior run IS the retired input, there is no separate prior_retired cursor (CasBlobInDegree.cpp:380-389). Corrected to 'two-cursor merge'. - CaCasMountCore.tla (x2): the reclaim comment cited CasServerRoot.cpp:405-414 for the successor-body-install putOverwrite, but that range is now inside the observation loop; the actual token-guarded makeMountBody + putOverwrite reclaim is at :331-332. Corrected the line reference. Comment-only; no code or model logic changed.
Two independent xhigh reviews (Fable + Codex gpt-5.6-sol) validated the bounded-worker architecture (all prior blocking flaws closed). Folded in the exception-safety findings: preallocated no-throw task/outcome/exception slots + non-throwing drain ordered before rollback (replaces the wrong SCOPE_EXIT wording); outcome published inside the appendRefOps confirm before any throwable post-commit work; dedicated CAS commit pool with a real bound (not a shared-IO fan-out); mandatory st.build reset after abandon; non-owning worker captures; and a bundled dependency to fix a latent ref-lane leader-exception use-after-free that concurrent workers activate. Plus join-ordering / hardlink / ref-lane tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ning-before-concurrency) Order: ref-lane exception-safety (bundled dep) -> exact-manifest CommitOutcome + dropRefIfMatches -> sequential rollback rework + st.build reset -> setting + dedicated commit pool -> parallelize commit (bounded worker-loops, exception-safe drain) -> benchmark + BACKLOG close-out. Correctness machinery lands before concurrency flips on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…on-throw
Reconciled fix batch from a double xhigh review (Fable + gpt-5.6-sol) of the
E5 uniquely-owned-base materialize.
A. Reachability: flushRefBatch's trial-validation copy `working` shared the
live base and outlived the post-PUT install, so materializeCommitted always
saw use_count()==2 and copied every row (O(N)). Release `working` before id
allocation -> the live base is uniquely owned at install -> in-place
O(overlay) fold. Same thread, so the decrement is program-order-before the
fold's use_count() load; no lock needed.
B. Race close: trySnapshotPublishOnce's `candidate_state = rt->state` (a
shared-base copy) was destroyed off-lock on the publisher thread; its
shared_ptr release-decrement raced the relaxed use_count() load the flush
thread's in-place fold now performs. Reset candidate_state under state_mutex
right after snapshotOf (covers every exit path). Both COW headers' safety
argument extended: every cross-thread copy is created AND destroyed under the
state lock, so a use_count() of 1 under that lock is stable both ways.
C. Coherence-on-throw: both containers' in-place fold is now incrementally
coherent -- per overlay entry the base mutation (its only throw point) runs
first, then the non-throwing net_delta adjust and overlay.erase. At every
escape point (base, overlay, net_delta) is exactly coherent (size() exact,
merged view unchanged, resumable) -- stronger than the copy path, load-bearing
because materialize runs after a durable PUT. The Committed-arm catch is split:
a materialize OOM is swallowed (durable, applied, coherent -- not a bricked
lane) and the "recovery re-hits" framing is scoped to applyRefLogTxn honestly;
tail-counter bumps moved before the fold so a fold failure can't skip them.
New randomized fast-vs-forced-slow parity tests pin both containers.
F. Wedge follow-ups: after resolving a wedge, materializeCommitted folds the
applied overlay in place and maybeScheduleSnapshotPublish runs so an all-no-op
or empty carve can't leave the table over-threshold until the next op. New
WedgeResolutionJoinsTailCountersAndFoldsOverlay pins the tail-counter
transitions across unresolved/resolved/ordinary.
G2. manifestEdgesOfTxn reserves txn.ops.size().
G3. "immutable base" doc sweep across both COW headers, materializeCommitted,
and the ledger install comment: base is immutable WHILE SHARED, mutable in
place when uniquely owned under the state lock.
G4. New ProfileEvents CasRefMaterializeInPlace / CasRefMaterializeCopy,
incremented in each container's materialize path, so a soak can prove the
fast path fires in production.
G1. UnrecognizedOwnerTransitionShapeAbortsRoundNeverDeletes appends a legal
foldable log to ns after capturing cursor_before, making the round-wide-abort
cursor pin load-bearing.
Gate 1116/1116 (1113 + 3 net-new), 0 failures. Bench: BM_FlushInstallUniqueOwner
1654ns@100 -> 13355ns@100k (~1727x over BM_FlushInstall at 100k); writer-path
benches within noise of the Final block.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Y1husTcX5kgM8YjpcDxeM
The checkTableCanBeRenamed forward on StorageTableProxy (7ab1fc1) materializes the lazy table via getNested while DatabaseAtomic holds its non-recursive database mutex. A schema-inferred lazy Buffer resolves its destination via DatabaseCatalog::getTable in its constructor, re-entering the same database and self-deadlocking (cross-database RENAME/EXCHANGE can hold two database mutexes across the same work). Found by the xhigh review (codex). Remove the override; the generic checkMutationIsPossible forward on StorageProxy is sound and kept. The nested engine's rename restriction is once again bypassed for a lazy (never-accessed) table -- a pre-existing gap, now reopened and tracked in the CAS backlog; the correct fix is interpreter-level materialization before the database mutex is taken. The backlog note also flags codex F5: the kept generic mutation forward changes StorageTableFunctionProxy mutation-check semantics -- call it out in any upstream PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Y1husTcX5kgM8YjpcDxeM
Phase 1/2 setup duplicated the database/table bring-up and still created the DB with lazy_load_tables = 1, while phase 3's shared helper used CREATE DATABASE IF NOT EXISTS without the setting -- a silent no-op on a reused stand that inherited a prior run's lazy DB, so the quarantined StorageTableProxy path kept running non-deterministically (found by the xhigh review, codex). Unify every phase onto setup_cluster_and_table, which now drops+recreates the dedicated (empty) soak database EAGERLY so lazy_load_tables is always exactly what we specify. The table is torn down first with a generous timeout so the DROP DATABASE that follows is trivial. Remove the last SETTINGS lazy_load_tables and fix the stale comments/log lines (module DB constant, the tables_loaded health gate, both "(lazy_load_tables=1)" logs, and an incidental mention in cluster.py). python3 -m py_compile clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Y1husTcX5kgM8YjpcDxeM
…hurn level cap RCA of the t8v2 false red (169,837 AwaitingGc at t+420 -> drained to 14 by run end through the normal condemn->graduate->delete pipeline; GC 46 Success rounds, zero anomalies): mid-churn unreachable is a fold backlog that scales with churn rate while reachable sits at its floor, so the old '50*reachable+100000' cap is a guaranteed false trip on any churny run. Strict check moved to the final converge checkpoint (post-quiesce, pipeline grace elapsed) with a pipeline-floor bound; mid-run keeps tracking only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…2073 sanitizer abort)
…73 sanitizer abort)
…onnull UB (PR#2073, STID 5930-5afa)
…s + pr2073 stabilization complete Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # .github/workflows/master.yml # .github/workflows/pull_request.yml # .github/workflows/pull_request_community.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
content addressable storage - draft PR
Documentation entry for user-facing changes
TBD.
Exclude tests:
Regression jobs to run: